Hotfix to enhance code clarity, and update configurations for high availability (HA) systems #69
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ailability (HA) systems
devanshjainms
requested review from
KimForss,
Copilot,
hdamecharla and
mkdeegan
and removed request for
mkdeegan
There was a problem hiding this comment.
Pull Request Overview
This PR enhances code clarity and updates resource configurations for high availability systems in SAP HANA deployments on Azure by improving filesystem handling, refining resource properties, and updating test case setups. Key changes include updates to filesystem freeze methods, enhanced resource configuration defaults, and modifications to test commands and validation logic.
Reviewed Changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/roles/ha_db_hana/resource_migration_test.py | Updated command templating and added a new resource ID retrieval command for HANA. |
| tests/roles/ha_db_hana/block_network_test.py | Replaced ping with nc for network blocking tests and updated in-code comments accordingly. |
| tests/modules/get_pcmk_properties_db_test.py | Added sensitive parameters for testing scenario, ensuring they are skipped during processing. |
| tests/modules/filesystem_freeze_test.py | Adjusted mount point expectations in filesystem freeze tests. |
| src/vars/input-api.yaml | Updated default values and command templating for HANA DB HA test cases. |
| src/roles/misc/tasks/test-case-setup.yml & rescue.yml | Minor updates in variable setups for improved clarity in test case configuration. |
| src/roles/ha_db_hana/tasks/resource-migration.yml | Introduced a new block to retrieve HANA resource id with fallback handling. |
| src/roles/ha_db_hana/tasks/files/constants.yaml | Updated resource default timeout and delay values, and renamed resource key for clarity. |
| src/roles/ha_db_hana/tasks/block-network.yml | Modified network connectivity check to use nc instead of ping. |
| src/module_utils/commands.py | Modified the FREEZE_FILESYSTEM lambda to accept a mount point parameter. |
| docs/HIGH_AVAILABILITY.md | Revised documentation instructions with updated documentation links. |
| src/modules/get_pcmk_properties_scs.py, src/modules/get_pcmk_properties_db.py | Updated parsing logic to skip sensitive data in NV pair elements and refined resource configuration retrieval. |
| src/modules/filesystem_freeze.py | Adjusted _find_filesystem to return a tuple and updated corresponding command invocations. |
| src/modules/check_indexserver.py | Updated expected properties structure for SUSE to a list format for consistency. |
Files not reviewed (1)
- tests/roles/mock_data/cibadmin.txt: Language not supported
Comments suppressed due to low confidence (1)
src/roles/ha_db_hana/tasks/files/constants.yaml:62
- The value for pcmk_delay_max was changed from '30s' to '15' without a unit. Verify that this numeric value is processed as intended downstream and that the absence of a unit does not cause issues.
pcmk_delay_max: "15"
…g bash as the shell executable
devanshjainms
added
bug
KimForss
reviewed
Apr 22, 2025
KimForss
reviewed
Apr 22, 2025
… in HA configuration
KimForss
previously approved these changes
Apr 22, 2025
dennispadia
approved these changes
Apr 22, 2025
devanshjainms
added a commit
to devanshjainms/sap-automation-qa
that referenced
this pull request
Apr 22, 2025
…ailability (HA) systems (Azure#69)
devanshjainms
added a commit
to devanshjainms/sap-automation-qa
that referenced
this pull request
Apr 22, 2025
…ailability (HA) systems (Azure#69)
devanshjainms
added a commit
that referenced
this pull request
Apr 28, 2025
…ailability (HA) systems (#69)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request introduces several updates across multiple files, focusing on improving functionality, enhancing configurability, and addressing issues in high availability (HA) setups for SAP HANA on Azure. Key changes include updates to filesystem handling, resource configurations, and test case setups.
Filesystem and Mount Handling:
_find_filesystemmethod insrc/modules/filesystem_freeze.pyto return both the filesystem and mount point, improving clarity and enabling more precise operations.FREEZE_FILESYSTEMcommand insrc/module_utils/commands.pyto accept a mount point as a parameter, aligning with the updated_find_filesystemmethod.Resource Configuration Enhancements:
src/modules/get_pcmk_properties_db.pyto include specific types likesbd_stonithandfence_agentfor better granularity.RESOURCE_DEFAULTSinconstants.yamlto refine timeout values and add specific configurations forsbd_stonith. [1] [2]Test Case and Validation Improvements:
resource-migration.yml, improving flexibility across different setups.input-api.yaml, centralizing configuration and reducing duplication. [1] [2]Security and Data Handling:
_parse_nvpair_elementsin bothget_pcmk_properties_db.pyandget_pcmk_properties_scs.pyto skip sensitive parameters likepasswdandpassword, enhancing security. [1] [2]Documentation and Miscellaneous:
HIGH_AVAILABILITY.mdto reference the correct documentation link.azure-mgmt-networkpackage inazure-lb.yml, likely due to redundancy or a shift in dependency management.